Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix annoying areYouSure confirm caused by Chrome auto filling (failed trial) #17267

Closed
wants to merge 2 commits into from

Conversation

wxiaoguang
Copy link
Contributor

@wxiaoguang wxiaoguang commented Oct 8, 2021

Closes #16861

Chrome always auto fills some inputs, and triggers areYouSure confirm when leaving, autocomplete=off doesn't help at all (see the screenshots in the issue above, even if there is autocomplete=off, Chrome still auto fills)

So we can just wait for a few seconds, then the changes of the inputs are sure to be caused by real users.

Tested and it works as expected.

@wxiaoguang wxiaoguang added type/bug topic/ui Change the appearance of the Gitea UI labels Oct 8, 2021
// so we can just wait for a few seconds, then the changes of the inputs are sure to be caused by real users.
setTimeout(() => {
$('form:not(.ignore-dirty)').areYouSure();
}, 3000);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't something like 1 second shouldn't be enough? 3 seconds seems a bit too much imho

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it doesn't differ much for real users. 3 second is safer to make Chrome do its private operations.

No user would complete a complex form in 3 seconds. If he/she can complete a form in 3 seconds, then he/she won't worry about the lost of the content because the content can be re-filled in another 3 seconds very soon. 😂

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but if you edit form it could be also simple checkbox that needs to be changed

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, I will try more, and I find a new problem about auto filling ...

@GiteaBot GiteaBot added lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Oct 8, 2021
@lafriks lafriks added this to the 1.16.0 milestone Oct 8, 2021
@wxiaoguang wxiaoguang added the pr/wip This PR is not ready for review label Oct 8, 2021
Comment on lines +2972 to +2973
// Chrome always auto fills some inputs, and triggers `areYouSure` confirm when leaving, `autocomplete=off` doesn't help at all.
// so we can just wait for a few seconds, then the changes of the inputs are sure to be caused by real users.
Copy link
Member

@silverwind silverwind Oct 8, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// Chrome always auto fills some inputs, and triggers `areYouSure` confirm when leaving, `autocomplete=off` doesn't help at all.
// so we can just wait for a few seconds, then the changes of the inputs are sure to be caused by real users.
// Prevent confirm dialog from triggering on browser auto-fill

Autofill is not Chrome-specific and I think this comment is descriptive enough

// so we can just wait for a few seconds, then the changes of the inputs are sure to be caused by real users.
setTimeout(() => {
$('form:not(.ignore-dirty)').areYouSure();
}, 1000);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
}, 1000);
}, 2000);

Maybe a better compromise. I for example use BitWarden to autofill and it certainly has a bit of delay to it where 1s might not suffice. If we could somehow detect autofill, that would be an even better option but I'm not sure if it's possible.

@silverwind
Copy link
Member

silverwind commented Oct 8, 2021

Autofill detection might be possible by monitoring for an input event which I assume would only fire on actual user input, but it needs to be tested. I think it might be possible to only register the areYouSure handler after the form has received its first input event.

@wxiaoguang
Copy link
Contributor Author

wxiaoguang commented Oct 8, 2021

I meet a new problem. Chrome is really annoying. I am thinking about a totally new solution ....

    // Chrome always auto-fills some inputs, and triggers `areYouSure` confirm when leaving, `autocomplete=off` doesn't help at all.
    // When a page is just loaded, JavaScript can not read auto-filled fields before a user interaction occurs,
    // then the problem becomes:
    //   1) A page gets loaded, users can saw fake auto-filled input values, JavaScript can only get empty strings for the auto-filled inputs.
    //   2) When user clicks or inputs, Chrome auto-fills real input values. Then JavaScript can read real values.

@silverwind
Copy link
Member

Maybe https://github.com/matteobad/detect-autofill is useful, but it looks to be quite a hack.

@wxiaoguang
Copy link
Contributor Author

After some investigation, the solutions all seem very hacky, so I think we had better close this PR and to see whether there are better solutions.

@wxiaoguang wxiaoguang closed this Oct 8, 2021
@wxiaoguang wxiaoguang changed the title Fix annoying areYouSure confirm caused by Chrome auto filling Fix annoying areYouSure confirm caused by Chrome auto filling (failed trial) Oct 8, 2021
@wxiaoguang wxiaoguang deleted the fix-are-you-sure branch October 8, 2021 14:22
@go-gitea go-gitea locked and limited conversation to collaborators Apr 28, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. pr/wip This PR is not ready for review topic/ui Change the appearance of the Gitea UI type/bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

How do I close the annoying pop-up window?
4 participants